feat(font): Japanese UI P5 PR B — CJK font converter + parity gtest - #2
Merged
Conversation
Convert the fork's cjk_ui_font_20.h (3-byte-row MSB-first PROGMEM) into the
standard EpdFontData 1-bit tight-packed format, emitting a checked-in
cjk_ui_20.{h,cpp} pair (3420 glyphs, 169240 bitmap bytes, 2405 intervals).
The parser strips C/C++ comments before tokenising: the fork annotates each
glyph in CJK_UI_GLYPHS with a `// U+XXXX (c)` comment, which otherwise injects
spurious tokens and (via the literal `}` in the U+007D comment) truncates the
brace-body capture. A sha256 of the input source is embedded in both outputs so
CI can detect drift against the (un-checked-in) fork source.
Decode every covered codepoint from both the converted EpdFontData and the fork
reference (extracted via the shared parser) and assert pixel-identical bitmaps,
plus metric-floor, glyph-count and advanceX==width<<4 invariants.
The fork source is not checked in, so the target is guarded with
`if(NOT EXISTS ${FORK_HEADER}) return()` — it runs as a local-only verification
and is skipped where the fork is absent (e.g. standard CI). CI's drift guard is
the sha256 comment in the committed artifact.
Address review findings on PR B's verification posture: - Add CjkFontSelfTest (fork-independent, always built so it runs in CI): pins per-glyph metadata (height/top/left/advanceX/dataLength), bitmap dataOffset contiguity, and interval sorted/non-overlap/offset invariants. Pixel-only parity could not catch metadata or interval regressions, and the fork-gated parity test does not run in CI — this gives the committed artifact standing CI protection. Bit-per-bit pixel parity stays as the local fork-gated CjkFontParityTest. - Replace assert-based mandatory checks (stripped under `python -O`) with -O-safe sys.exit checks via a shared validate_arrays() helper, used by both the converter and the parity-reference extractor; add an empty-array guard. - Remove the redundant sample-only check_cjk_font_data.py — the exhaustive gtest fully subsumes it and it duplicated the parser. - Correct the CMakeLists comment that overclaimed an automated CI sha256 drift guard (the sha256 is a marker for manual/converter-rerun drift detection). - Fix clang-format wrap in CjkFontParityTest; reword a stale extractor comment and drop the unused kForkRef_NumGlyphs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope — PR B (Tasks 2–3 of Japanese UI Support P5)
Builds the offline tooling and the converted 20px CJK UI font asset, plus a host gtest proving bit-perfect parity against the fork source. This is purely additive: it creates the new P5 artifacts and does not touch the existing P4 fallback machinery (deleted later in PR C). P4 and the new
cjk_ui_20.{h,cpp}coexist after this PR by design.Task 2 — Python converter + generated asset
scripts/convert_cjk_font_to_epdfontdata.py— converts the fork's 3-byte-row MSB-first PROGMEM font into standardEpdFontData1-bit tight packing.scripts/check_cjk_font_data.py— manual sanity tool (sampled bit comparison).lib/EpdFont/builtinFonts/cjk_ui_20.{h,cpp}— generated & committed: 3420 glyphs, 169 240 bitmap bytes, 2405 intervals. Metrics:advanceY=24, ascender=16, descender=4, is2Bit=false; per-glyphadvanceX = width<<4,top=16,height=20.sha256:of the input source is embedded in both outputs so drift against the (un-checked-in) fork source is detectable.Task 3 — host gtest, bit-per-bit parity
test/cjk_font_parity/—CjkFontParityTest.cpp+CMakeLists.txt, wired intotest/CMakeLists.txt.EpdFontDataand an independently-extracted fork reference (scripts/extract_cjk_fork_reference.py, shared parser) and asserts pixel-identical bitmaps, plus metric-floor / glyph-count /advanceX==width<<4invariants.Necessary deviations from the plan (the plan's literal code fails against the real fork header)
CJK_UI_GLYPHSwith a// U+XXXX (c)comment. Without stripping, (a)int('0021', 0)raises on theU+0021text and (b) the literal}in theU+007D (})comment truncates the brace-body capture (only 87 glyphs parsed). Stripping first yields the correct 3420×60 = 205 200 source bytes. Output format is unchanged from the plan.check_cjk_font_data.pybracket regex accepts a sized array ([169240], the generated output) in addition to[](the fork source).if(NOT EXISTS ${FORK_HEADER}) return(). CI'sunit-testsjob builds the full test suite viacmake -S test, but the fork source lives only at a local/tmppath (not checked in). Without the guard CI's build would break. The parity test is a local correctness gate; CI's drift guard is the embeddedsha256:comment in the committed artifact.Verification (host-side — this machine has no PlatformIO;
pio run/ clang-format deferred to CI)Converting 3420 glyphs… 169240 bitmap bytes, 2405 intervals.check_cjk_font_data.py: all 22 sampled codepoints PASS.AllCodepointsPixelPerfectover all 3420 codepoints. Full suite: 103/103 passed.AllCodepointsPixelPerfectred; restored via regeneration..cppcompiles clean under-Wall -Wextra -pedantic -std=c++20. (lib/EpdFont/builtinFonts/is excluded from clang-format and is not yet referenced by firmware, so no flash delta in this PR — the ~169 KB lands when the asset is wired in by a later PR.)